Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tarball on rocm tags and MAINTAINERS guide #402

Merged
merged 10 commits into from
Sep 24, 2024
Merged

Tarball on rocm tags and MAINTAINERS guide #402

merged 10 commits into from
Sep 24, 2024

Conversation

coleramos425
Copy link
Collaborator

@coleramos425 coleramos425 commented Aug 9, 2024 •
edited
Loading

This PR adds a MAINTAINERS.md file which outlines the procedure for publishing an Omniperf release. The guide should be shared with any project maintainers who will be publishing releases.

I've also modified the packaging.yml action such that any tags matching regex rocm-[0-9]+.[0-9]+.[0-9]+* will trigger the workflow which produces our release tarball.

TODO:

  • Once @dgaliffiAMD has published the amd-mainline and amd-staging branches, update the empty links in the MAINTAINERS.md file
  • Solicit feedback from devops team to ensure expectations are clearly communicated

@coleramos425 coleramos425 requested review from a team and koomie as code owners August 9, 2024 17:33
@coleramos425
Copy link
Collaborator Author

@koomie please feel free to add any feedback if there's anything I missed.

@dgaliffiAMD dgaliffiAMD requested a review from WBobby August 9, 2024 17:46
dgaliffiAMD
dgaliffiAMD reviewed Aug 9, 2024
View reviewed changes
Copy link
Collaborator

@dgaliffiAMD dgaliffiAMD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Looks good. Just some minor formatting changes.

MAINTAINERS.md Outdated Show resolved Hide resolved
MAINTAINERS.md Outdated Show resolved Hide resolved
MAINTAINERS.md Outdated Show resolved Hide resolved
@coleramos425
Copy link
Collaborator Author

coleramos425 commented Aug 16, 2024 •
edited
Loading

Some feedback from ROCm devops team on this PR (via @WBobby)

  1. CHANGELOG formatting must adhere to standard ROCm formatting for "rocm-ci" to correctly detect and copy changes to release notes. He provided this as a CHANGELOG template (https://github.com/ROCm/hipTensor/blob/develop/CHANGELOG.md)
  2. "rocm-ci" bot will not copy the release tarball from Omniperf's packaging action. It's on us to come up with a workaround for this

TODO

  • I can address (1) fairly easily with some minor changes to this PR (also related to Move CHANGES to CHANGELOG.md #410).
  • For (2) I'll need to consult with the team on the best way to proceed.

@coleramos425 coleramos425 force-pushed the colramos/release-info branch 2 times, most recently from 56cf357 to 8395f67 Compare August 20, 2024 18:35
@coleramos425
Copy link
Collaborator Author

coleramos425 commented Aug 20, 2024 •
edited
Loading

I've modified the packaging workflow so that on a new tag (including tags matching the format rocm-x.x.x) the workflow will upload the release tarball to the corresponding release (in case of rocm-x.x.x tag, the release name "Omniperf vX.X.X for rocm-x.x.x").

omniperf/.github/workflows/packaging.yml

Lines 48 to 62 in 7441f04

- name: Determine release name
run: |
if [[ ${{github.ref_name}} == rocm-* ]]; then
echo "RELEASE_NAME=Omniperf ${{ env.VERSION }} for ${{github.ref_name}}"
else
echo "RELEASE_NAME=Omniperf ${{ env.VERSION }}"
fi >> $GITHUB_ENV
- name: Upload tarball Release Asset
uses: softprops/action-gh-release@v1
if: startsWith(github.ref, 'refs/tags/') && github.repository == 'ROCm/omniperf'
with:
fail_on_unmatched: True
draft: True
name: ${{ env.RELEASE_NAME }}
files: |

This should comply with requirements from the DevOps team and their "rocm-ci" bot used to trigger releases.

@coleramos425 coleramos425 added the documentation Improvements or additions to documentation label Aug 22, 2024
@coleramos425
Copy link
Collaborator Author

After meeting with @arjun-raj-kuppala we've decided that instead of triggering the packaging workflow on tag, we should trigger on release and then upload the subsequent tarball artifact to the latest release as an asset. It's a little different than what I'm doing now, but should be an easy change.

Other than that, this PR complies on his end.

@coleramos425
Update softprops pluvin to v2 and remove release name
e8862b9 
Removing the release name enables us to modify an existing release with the matching tag

Signed-off-by: coleramos425 <[email protected]>
@coleramos425
Remove draft option on softprops plugin
ccc9fdf 
Signed-off-by: coleramos425 <[email protected]>
@coleramos425 coleramos425 mentioned this pull request Sep 24, 2024
Merged
@coleramos425 coleramos425 merged commit 6265391 into amd-staging Sep 24, 2024
12 of 14 checks passed
@coleramos425 coleramos425 deleted the colramos/release-info branch September 24, 2024 21:17
coleramos425 added a commit that referenced this pull request Sep 25, 2024
@coleramos425
Tarball on rocm tags and MAINTAINERS guide (#402)
0d15023 
* Package release tarball on rocm tags and add a MAINTAINERS guide to outline release procedure

Signed-off-by: coleramos425 <[email protected]>

* Apply linting feedback

Signed-off-by: coleramos425 <[email protected]>

* Update links in MAINTAINERS for new branches

Signed-off-by: coleramos425 <[email protected]>

* Update MAINTAINERS.md for changelog formatting

Signed-off-by: coleramos425 <[email protected]>

* Update Omniperf ver checker to allow tags with rocm- prefix

Signed-off-by: coleramos425 <[email protected]>

* Add step to packaging workflow to upload release tarball to release

Signed-off-by: coleramos425 <[email protected]>

* Synax change for release name in packaging action

Signed-off-by: coleramos425 <[email protected]>

* Update MAINTAINERS for instuctions on ROCm releases

Signed-off-by: coleramos425 <[email protected]>

* Update softprops pluvin to v2 and remove release name

Removing the release name enables us to modify an existing release with the matching tag

Signed-off-by: coleramos425 <[email protected]>

* Remove draft option on softprops plugin

Signed-off-by: coleramos425 <[email protected]>

---------

Signed-off-by: coleramos425 <[email protected]>
coleramos425 added a commit that referenced this pull request Sep 27, 2024
@coleramos425
Tarball on rocm tags and MAINTAINERS guide (#402)
d2e806d 
* Package release tarball on rocm tags and add a MAINTAINERS guide to outline release procedure

Signed-off-by: coleramos425 <[email protected]>

* Apply linting feedback

Signed-off-by: coleramos425 <[email protected]>

* Update links in MAINTAINERS for new branches

Signed-off-by: coleramos425 <[email protected]>

* Update MAINTAINERS.md for changelog formatting

Signed-off-by: coleramos425 <[email protected]>

* Update Omniperf ver checker to allow tags with rocm- prefix

Signed-off-by: coleramos425 <[email protected]>

* Add step to packaging workflow to upload release tarball to release

Signed-off-by: coleramos425 <[email protected]>

* Synax change for release name in packaging action

Signed-off-by: coleramos425 <[email protected]>

* Update MAINTAINERS for instuctions on ROCm releases

Signed-off-by: coleramos425 <[email protected]>

* Update softprops pluvin to v2 and remove release name

Removing the release name enables us to modify an existing release with the matching tag

Signed-off-by: coleramos425 <[email protected]>

* Remove draft option on softprops plugin

Signed-off-by: coleramos425 <[email protected]>

---------

Signed-off-by: coleramos425 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgaliffiAMD dgaliffiAMD dgaliffiAMD approved these changes

@koomie koomie Awaiting requested review from koomie koomie is a code owner

@WBobby WBobby Awaiting requested review from WBobby

@arjun-raj-kuppala arjun-raj-kuppala Awaiting requested review from arjun-raj-kuppala

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@coleramos425 @dgaliffiAMD